Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core): add workspaces path if package path is not included #28824

Merged
merged 1 commit into from
Dec 6, 2024
Merged

fix(core): add workspaces path if package path is not included #28824

merged 1 commit into from
Dec 6, 2024

Conversation

xiongemi
Copy link
Collaborator

@xiongemi xiongemi commented Nov 6, 2024 •
edited
Loading

Current Behavior

  • when import to a path where is not in the workspaces, it currently just shows a warning. however, it will cause an error like "module not found" because there are packages not installed.

Expected Behavior

  • automatically add to the workspaces
    Screenshot 2024-11-08 at 12 58 42 AM

Related Issue(s)

Fixes #

@xiongemi xiongemi requested a review from a team as a code owner November 6, 2024 21:38
@xiongemi xiongemi requested a review from FrozenPandaz November 6, 2024 21:38
@vercel Vercel
Copy link

vercel bot commented Nov 6, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Dec 6, 2024 7:24am

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Nov 6, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ae74ec7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 6 targets

Sent with 💌 from NxCloud.

@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch 9 times, most recently from e594984 to 0cf563d Compare November 8, 2024 15:41
@xiongemi xiongemi requested review from Coly010 and a team as code owners November 8, 2024 15:41
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch 2 times, most recently from 356f0d7 to 86c4253 Compare November 8, 2024 17:54
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 86c4253 to 2e4b8ae Compare November 14, 2024 22:04
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 2e4b8ae to 49bef64 Compare November 14, 2024 23:22
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 49bef64 to 781cf08 Compare November 15, 2024 03:44
@xiongemi xiongemi requested review from meeroslav, vsavkin, mandarini and a team as code owners November 15, 2024 03:44
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from a1e76cb to 8156a5d Compare November 20, 2024 05:49
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 8156a5d to 65d614f Compare November 20, 2024 17:24
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 65d614f to 342bfff Compare November 21, 2024 02:13
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 342bfff to 0625b8a Compare November 21, 2024 18:19
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 0625b8a to e5672f8 Compare November 25, 2024 17:56
FrozenPandaz
FrozenPandaz requested changes Nov 26, 2024
View reviewed changes
packages/nx/src/command-line/import/import.ts Outdated Show resolved Hide resolved
packages/nx/src/command-line/import/import.ts Outdated Show resolved Hide resolved
packages/nx/src/command-line/import/import.ts Outdated Show resolved Hide resolved
packages/nx/src/command-line/init/init-v2.ts Outdated Show resolved Hide resolved
packages/nx/src/utils/package-manager.ts Show resolved Hide resolved
packages/nx/src/utils/package-manager.ts Outdated Show resolved Hide resolved
packages/nx/src/utils/package-manager.ts Outdated Show resolved Hide resolved
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch 2 times, most recently from 0cd3e68 to c6c1811 Compare November 28, 2024 20:17
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from c6c1811 to 51cb644 Compare November 28, 2024 22:42
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 51cb644 to b3291b6 Compare November 28, 2024 23:55
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from b3291b6 to 4438b44 Compare November 29, 2024 05:47
@xiongemi xiongemi force-pushed the fix/not-import-uninstall-deps branch from 4438b44 to ae74ec7 Compare December 6, 2024 07:16
@xiongemi xiongemi merged commit 902eef5 into master Dec 6, 2024
6 checks passed
@xiongemi xiongemi deleted the fix/not-import-uninstall-deps branch December 6, 2024 16:25
FrozenPandaz pushed a commit that referenced this pull request Dec 6, 2024
@xiongemi @FrozenPandaz
fix(core): add workspaces path if package path is not included (#28824)
046686f 
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- when import to a path where is not in the workspaces, it currently
just shows a warning. however, it will cause an error like "module not
found" because there are packages not installed.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
- automatically add to the workspaces
![Screenshot 2024-11-08 at 12 58
42 AM](https://github.com/user-attachments/assets/d36dd093-a0ce-45c3-a783-97244741971f)

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

(cherry picked from commit 902eef5)
@github-actions GitHub Actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@FrozenPandaz FrozenPandaz FrozenPandaz approved these changes

@Coly010 Coly010 Awaiting requested review from Coly010

@meeroslav meeroslav Awaiting requested review from meeroslav

@vsavkin vsavkin Awaiting requested review from vsavkin

@mandarini mandarini Awaiting requested review from mandarini

@jaysoo jaysoo Awaiting requested review from jaysoo

Assignees

@xiongemi xiongemi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xiongemi @jaysoo @FrozenPandaz